Skip to content

Issue215#228

Merged
StefH merged 5 commits intozzzprojects:masterfrom
david-garcia-garcia:ISSUE215
Nov 25, 2018
Merged

Issue215#228
StefH merged 5 commits intozzzprojects:masterfrom
david-garcia-garcia:ISSUE215

Conversation

@david-garcia-garcia
Copy link
Copy Markdown
Contributor

@david-garcia-garcia david-garcia-garcia commented Nov 25, 2018

2 Changes in this PR.

  • DynamicClassFactory.cs no longer creates a parameter constructor when the class has no properties. Prior to this, a class with a duplicate empty constructor was created.

  • Added DisableMemberAccessToIndexAccessorFallback to disable automatic downcast/fallback to item accesor when a member is not found. This is an enhancement for developers because it helps early detection of badly created expressions that will fail to evaluate. Previous behaviour is respected as this is a new optional flag.

Copy link
Copy Markdown
Collaborator

@StefH StefH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor textual changes

Comment thread src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs Outdated
if (!_parsingConfig.DisableMemberAccessToIndexAccesorDowncast)
{
return Expression.Call(instance, indexerMethod, Expression.Constant(id));
MethodInfo indexerMethod = instance.Type.GetMethod("get_Item", new[] {typeof(string)});
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing some spaces here, did you do a format-file?

public bool RenameParameterExpression { get; set; } = false;

/// <summary>
/// By default when a member is not found in a type and the type has a string bassed index accessor it will be parsed as an index accesor. Use
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based

public bool RenameParameterExpression { get; set; } = false;

/// <summary>
/// By default when a member is not found in a type and the type has a string bassed index accessor it will be parsed as an index accesor. Use
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double 's'

/// <summary>
/// By default when a member is not found in a type and the type has a string bassed index accessor it will be parsed as an index accesor. Use
/// this flag to disable this behaviour and have parsing fail when parsing an expression
/// where a member access on a non existing member happens
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// where a member access on a non existing member happens
/// where a member access on a non existing member happens. Default value is false.

Comment thread test/System.Linq.Dynamic.Core.Tests/EntitiesTests.Select.cs
Comment thread test/System.Linq.Dynamic.Core.Tests/EntitiesTests.Select.cs
@StefH
Copy link
Copy Markdown
Collaborator

StefH commented Nov 25, 2018

It seems I did forget the CI Pull Request build integration with Azure Pipelines. I did change this and I hope that when you push new code, the build will trigger...

@StefH StefH added the feature label Nov 25, 2018
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 25, 2018

Codecov Report

Merging #228 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
+ Coverage   96.63%   96.63%   +<.01%     
==========================================
  Files          41       41              
  Lines        6648     6654       +6     
==========================================
+ Hits         6424     6430       +6     
  Misses        224      224
Impacted Files Coverage Δ
...ystem.Linq.Dynamic.Core/Parser/ExpressionParser.cs 98.58% <100%> (ø) ⬆️
...rc/System.Linq.Dynamic.Core/DynamicClassFactory.cs 100% <100%> (ø) ⬆️
src/System.Linq.Dynamic.Core/ParsingConfig.cs 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c814eb2...5141da3. Read the comment docs.

public bool RenameParameterExpression { get; set; } = false;

/// <summary>
/// By default when a member is not found in a type and the type has a string based index accessor it will be parsed as an index accesor. Use
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accessor

@StefH
Copy link
Copy Markdown
Collaborator

StefH commented Nov 25, 2018

1 more typo in accessor

still a typo in based

@david-garcia-garcia
Copy link
Copy Markdown
Contributor Author

Fixed the typo in accessor, still not sure what the typo in "based" is.

@StefH
Copy link
Copy Markdown
Collaborator

StefH commented Nov 25, 2018

all ok

@StefH
Copy link
Copy Markdown
Collaborator

StefH commented Nov 25, 2018

linked to #215

@StefH StefH added the bug label Nov 25, 2018
@StefH StefH merged commit 1ee8997 into zzzprojects:master Nov 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants